applecontainer: add Swift bridge skeleton and runtime stub#58
Conversation
First PR of M6 (apple-container runtime backend). Lands the applecontainer-bridge/ SwiftPM dynamic library and the runtime/applecontainer/ Go package as a skeleton — New + Ping only, every other Runtime method returns ErrNotImplemented for PR-B onward to fill in. Bridge (Swift): - Pinned exact to apple/container 0.12.3. - @_cdecl exports: ac_version, ac_ping(timeout) returning JSON-encoded SystemHealth, ac_free. Runtime (Go, darwin && arm64): - New probes ClientHealthCheck.ping; returns *runtime.DaemonUnavailableError if unreachable. - Ping exposed as a method so consumers can re-probe a live runtime. - Compile-time runtime.Runtime interface assertion catches upstream signature changes. - Non-darwin/arm64 build tag links a stub so the package imports cleanly cross-platform. Distribution: PR-A links the dylib at build time via cgo LDFLAGS rpath into applecontainer-bridge/.build/.../release. Run `make bridge` before any Go build that links runtime/applecontainer on darwin/arm64. go:embed + dlopen single-binary packaging lands in PR-A2. Test plan (run on darwin/arm64 with `container system start` first): - `make bridge && go test ./runtime/applecontainer/...` Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a darwin/arm64 Apple Container runtime that links to a new Swift C-bridge (ACBridge) for daemon health checks and version info, includes cgo wiring, runtime types and Ping/BridgeVersion implementations, runtime method stubs, Makefile build targets, ignore rules, and darwin/arm64 integration tests. ChangesApple Container Runtime for macOS arm64
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@applecontainer-bridge/Sources/ACBridge/bridge.swift`:
- Around line 14-29: ac_ping blocks indefinitely because sem.wait() is
unbounded; replace the plain sem.wait() with a timed wait using
DispatchSemaphore.wait(timeout:) based on the same timeoutSeconds (use the same
default 5s logic), check for .timedOut, and if timed out set a deterministic
error payload (e.g. call encodePingErr with a small PingTimeout Error type or
set json to a timeout JSON) before returning; refer to ac_ping,
DispatchSemaphore, Task, ClientHealthCheck.ping, encodePingOK and encodePingErr
to locate and update the logic.
In `@Makefile`:
- Around line 36-44: The Makefile targets bridge and bridge-clean currently
invoke swift unconditionally; update both targets (bridge and bridge-clean) to
first check the platform and architecture (e.g., uname -s and uname -m or
GOOS/GOARCH env) and only run the swift commands when on darwin and arm64,
otherwise treat the target as a no-op (print a short message or exit 0). Ensure
you change the bridge recipe (cd applecontainer-bridge && swift build -c
release) and bridge-clean recipe (cd applecontainer-bridge && swift package
clean && rm -rf .build) to be guarded by that platform check so they don't run
swift on unsupported systems.
In `@runtime/applecontainer/runtime_darwin_arm64.go`:
- Around line 81-86: The Ping method currently ignores ctx's deadline; change
Runtime.Ping to compute an effective timeout by checking ctx.Deadline() and
using the minimum of timeoutSeconds and the remaining time until the deadline
(rounding up to whole seconds), returning ctx.Err() immediately if the deadline
has already passed, then call C.ac_ping with that effective timeout (the
C.ac_ping invocation and cstr handling remain the same). Locate the Ping
function, replace the direct use of timeoutSeconds for C.ac_ping with the
computed effectiveTimeout (int32), and ensure you still respect ctx.Err() at the
top and before invoking C.ac_ping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb277a14-cb5d-424d-93e7-9703471b6e71
📒 Files selected for processing (10)
.gitignoreMakefileapplecontainer-bridge/.gitignoreapplecontainer-bridge/Package.swiftapplecontainer-bridge/Sources/ACBridge/bridge.swiftapplecontainer-bridge/include/ac_bridge.hruntime/applecontainer/doc.goruntime/applecontainer/runtime_darwin_arm64.goruntime/applecontainer/runtime_darwin_arm64_test.goruntime/applecontainer/runtime_unsupported.go
Three fixes from CodeRabbit review on #58, all valid: bridge.swift: ac_ping's `sem.wait()` was unbounded — if the Swift Task never signals (cancellation race, runtime hang) the cgo caller would block forever. ClientHealthCheck.ping already enforces its own Duration timeout, but we guard the semaphore as belt-and-suspenders with a deterministic "bridge-timeout" error payload. Makefile: bridge / bridge-clean recipes were unconditional but the comment claimed "no-op on other platforms." Guard with uname so invoking on linux/amd64 prints a clear skip message rather than failing with "swift: command not found." Comment rewritten to match. runtime_darwin_arm64.go: Ping ignored ctx.Deadline(). Clamp the effective bridge timeout to min(timeoutSeconds, ceil(time.Until( deadline).Seconds())) and return ctx.Err() if the deadline has already passed. The bridge call is synchronous from Go's view so we can't cancel mid-flight; bounding the argument is the closest we get to context semantics. Test plan: - `make bridge` still builds the dylib on darwin/arm64 - `make bridge` on non-darwin/arm64 prints "skipped" and exits 0 - `go test ./runtime/applecontainer/...` — 2 tests pass - `GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build` of stub — ok - `go vet ./runtime/applecontainer/...` — clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
First PR of M6 — a second
Runtimebackend that drives Apple'scontainerstack on macOS (Sequoia+, arm64). This PR lands the scaffolding only:
New+Ping. Every otherRuntimemethod returnsErrNotImplementeduntil PR-B onward.
Architecture: Go package cgo-links a Swift dynamic library
(
libACBridge.dylib) that importsapple/container'sContainerAPIClientand speaks XPC to the systemcontainer-apiserverdaemon. Same daemon model as Docker —
brew install container+container system startis the user's responsibility, analogous todockerdforruntime/docker.applecontainer-bridge/— SwiftPM dynamic-library package, exact-pinnedto
apple/container 0.12.3. Exportsac_version,ac_ping(timeout)(returns JSON-encoded
SystemHealth),ac_free.runtime/applecontainer/— Go package, build-taggeddarwin && arm64.NewprobesClientHealthCheck.ping; returns*runtime.DaemonUnavailableErroron failure.Pingis also exposed asa method so consumers can re-probe a live runtime. Compile-time
_ runtime.Runtime = (*Runtime)(nil)assertion catches upstreaminterface drift.
Newso thepackage imports cleanly cross-platform.
Makefilebridge+bridge-cleantargets.Deferred
PR-A links the dylib at build time via cgo
LDFLAGSrpath intoapplecontainer-bridge/.build/.../release. Single-binary packaging viago:embed+ dlopen lands in PR-A2.Test plan
Local verification (darwin/arm64 with daemon running):
brew install container && container system startmake bridgego test ./runtime/applecontainer/...— 2 tests pass:TestPing_DaemonRunninground-trips a realSystemHealththroughthe bridge;
TestNew_TypedErrorOnFailureasserts typed-errorcontract.
go test ./...— all existing tests still green; no regression.GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build ./runtime/applecontainer/...— stub compiles.go vet ./runtime/applecontainer/...clean.CI: this PR does not yet add a macOS arm64 runner. The Linux runner
exercises the stub path;
make bridge+ the daemon-dependent testsrun on a developer machine until the M6 CI job lands (PR-H).
Summary by CodeRabbit